Thanks for taking the time to review the pull request and suggest modifications to be made. I'll look into reducing the scope of changes in this pull request this week

Reduced the PR scope as requested in follow-up commit bcf42a4. This version keeps the GitHub CLI fallback (gh auth token --hostname ) ahead of git credential fill, and removes the repo-path threading, cache key expansion, and repo-path-specific tests/docs so the first batch stays focused. Focused auth validation still passes locally: ests/test_token_manager.py and ests/unit/test_auth.py.

Hi @awakecoding -- friendly check-in: this PR has been in changes-requested state since 2026-04-10, and we are working through the wave-3 merge backlog. If you are still able to address the review feedback in the next ~7 days, we would love to land this; otherwise we will close it as stale and you can re-open later (no hard feelings -- contribution graveyard happens to everyone).

Concretely, the outstanding feedback is:
Scope the change down for this security-critical auth path -- the current diff touches 8 files, alters the resolver cache key from (host, org) to (host, org, repo_path), and threads repo_path into git credential fill stdin (a newline-delimited protocol); reviewers asked to split / shrink before merge.

Let us know either way -- a quick "still on it" or "going to close it" reply is enough.

Thanks for the contribution!

Current branch scope is narrower than the original review summary described.

The current PR diff is 6 files and no longer includes the repo-path work that prompted the earlier concern:

• no repo_path threading into git credential fill
• no (host, org, repo_path) resolver cache key
• no install/marketplace plumbing for repo-context auth

What remains in this PR is the GitHub CLI fallback before git credential fill:

• GH_TOKEN remains in the GitHub module token chain
• gh auth token --hostname <host> is tried before git credential fill
• matching tests/docs were updated to reflect that narrower chain

Current changed files:

• src/apm_cli/core/token_manager.py
• src/apm_cli/core/auth.py
• tests/test_token_manager.py
• tests/unit/test_auth.py
• docs/src/content/docs/getting-started/authentication.md
• packages/apm-guide/.apm/skills/apm-usage/authentication.md

Focused validation still passes locally:

• $env:PYTHONPATH='.;src'; uv run python -m pytest tests/test_token_manager.py tests/unit/test_auth.py -x

If the remaining 6-file diff still feels too broad, I can split out the doc/test-only follow-up separately, but the repo-context auth changes are already removed from this PR.

APM Review Panel: ship_with_followups

PR #630 ships a clean zero-config gh CLI fallback for GitHub auth -- one blocking doc-table bug must land before merge, plus a cluster of correctness and test-coverage gaps to close in follow-ups.

cc @danielmeppiel -- a fresh advisory pass is ready for your review.

PR #630 delivers a meaningful ergonomic win: users who are already authenticated via gh auth login now get APM working out of the box on private GitHub packages without any additional env var configuration. The resolution-order change (env var -> gh auth token -> git-credential-fill) is architecturally correct and the subprocess invocation is structurally safe per supply-chain-security: list-form call prevents injection, FQDN guard constrains host, and the env var hardening is present. Auth-expert and python-architect converge on the same root problem -- divergent guard logic between auth.py (host_info.kind checks) and token_manager.py (_supports_gh_cli_host()) creates a latent GHES correctness divergence and a double-invocation bug when gh-auth-token is the resolved source and subsequently fails. These are real correctness defects but they are narrow-path and non-crashing; they belong in immediate follow-up issues, not as blockers to the PR itself.

One true blocking finding from doc-writer must be fixed before merge: the prose note inserted mid-table in packages/apm-guide/.apm/skills/apm-usage/authentication.md orphans the final table row, breaking rendering for all markdown parsers. This is a one-line fix (move the prose after the complete table) and is not controversial. The test-coverage-expert surfaces three missing-test gaps -- _supports_gh_cli_host untested, gh-auth-token source string never asserted, and the try_with_fallback gh-CLI branch entirely unreachable from the test suite due to the autouse disable fixture -- all three are missing-outcome findings on a secure-by-default surface. They should be filed as issues and resolved in a fast follow, not held as a merge gate given the narrow behavioral surface of this PR.

The oss-growth-hacker's framing is correct and strategically important: this PR is APM's clearest 'zero config for gh users' signal to date and deserves a CHANGELOG entry and social beat framed around 'if you use gh, you are already set up.' The authentication.md table inconsistency (prose says step 3, table labels priority 5) and the missing quickstart callout are both high-value, low-effort documentation fixes that should accompany or immediately follow this PR.

Dissent. No hard disagreements between panelists. DevX-UX and cli-logging-expert both flag the generic 'trying fallback credentials' log message -- the former from a developer troubleshooting lens, the latter from an operator observability lens. Both are right; the cli-logging-expert finding is the more actionable one (emit per-step outcome logs). Supply-chain-security rates the full-scope OAuth token concern as recommended (not blocking) because scope is ambient by design and cannot be narrowed at retrieval time; auth-expert's multi-account nit overlaps and is correctly classified at nit severity since it is pre-existing gh CLI behavior, not introduced by this PR.

Aligned with: Secure by default -- FQDN guard and list-form subprocess satisfy the bar; stdin=subprocess.DEVNULL is a cheap hardening win. Pragmatic as npm -- gh CLI now works as an implicit credential source with zero additional configuration, matching the zero-friction model users expect. Portable by manifest -- gh CLI fallback is host-scoped and additive; existing CI workflows using GH_TOKEN or GITHUB_TOKEN env vars are unaffected. OSS community driven -- reduces auth friction for the gh CLI cohort, lowering the barrier for new community adopters.

Growth signal. The oss-growth-hacker correctly identifies this as the strongest zero-config signal APM has shipped. The CHANGELOG entry should be framed as 'If you use gh, you are already set up' -- not as a technical resolution-order change, but as a user promise. A social beat (release note, README hero callout, quickstart one-liner) timed to the merge will convert the large gh CLI user cohort who previously bounced on APM's auth setup. The authentication.md table priority-numbering inconsistency should be fixed before that social beat lands, or it will undermine the claim.

Panel summary

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 5 follow-ups

1. [Doc Writer] (blocking-severity) Fix orphaned table row in apm-guide authentication.md by moving the prose note after the complete table -- Blocking rendering bug: the final | -- | None | row is detached from the table by the inserted paragraph in all standard markdown parsers. One-line fix; must land before merge.
2. [Test Coverage Expert] Add unit tests for _supports_gh_cli_host (None/ADO/github.com/GHES/mismatch branches) and assert ctx.source == 'gh-auth-token' when gh CLI supplies the token -- Missing-outcome evidence on a secure-by-default surface. _supports_gh_cli_host gates whether gh CLI is ever invoked; the autouse disable fixture makes the try_with_fallback gh-CLI branch entirely unexercised.
3. [Auth Expert] Add 'gh-auth-token' to _try_credential_fallback short-circuit guard and unify GHES guard logic between auth.py and token_manager.py -- Double subprocess invocation when gh-auth-token source fails, and latent GHES divergence between call-sites. Low blast radius but correctness defects on the auth critical path.
4. [Supply Chain Security Expert] Add stdin=subprocess.DEVNULL to the gh auth token subprocess call -- Cheap hardening: guarantees EOF on any interactive prompt attempt for older gh versions that ignore GH_PROMPT_DISABLED. One-line change, no behavior change for modern gh.
5. [OSS Growth Hacker] Fix authentication.md table priority-numbering inconsistency and add gh auth login callout to quickstart -- The table labeling gh auth token as priority 5 while prose says step 3 will confuse new users and undermine the zero-config narrative. Quickstart callout is the highest-conversion surface for the gh CLI cohort.

Architecture

classDiagram
    direction LR
    class AuthResolver {
      <<Strategy>>
      +resolve(host, org) AuthContext
      +try_with_fallback(host, org, operation) T
      -_resolve_token(host_info, org) tuple
      -_try_credential_fallback(exc) T
    }
    class GitHubTokenManager {
      <<ConcreteStrategy>>
      +resolve_credential_from_gh_cli(host) str
      +resolve_credential_from_git(host, port) str
      +get_token_with_credential_fallback(purpose, host) str
      -_supports_gh_cli_host(host) bool
      -_is_valid_credential_token(token) bool
    }
    class HostInfo {
      <<ValueObject>>
      +host str
      +kind str
      +display_name str
    }
    class AuthContext {
      <<ValueObject>>
      +token str
      +source str
      +host_info HostInfo
    }
    class GitHubTokenManager:::touched
    class AuthResolver:::touched
    AuthResolver *-- GitHubTokenManager : delegates credential resolution
    AuthResolver ..> HostInfo : reads kind/host
    AuthResolver ..> AuthContext : produces
    AuthContext *-- HostInfo : contains
    note for AuthResolver "Chain of Responsibility:\nenv-PAT -> env-token -> gh-auth-token (NEW) -> git-credential"
    note for GitHubTokenManager "_supports_gh_cli_host guard used only\nin get_token_with_credential_fallback;\nauth.py bypasses it with inline kind checks"
    classDef touched fill:#fff3b0,stroke:#d47600

flowchart TD
    A["CLI: apm install / run"] --> B["AuthResolver.resolve(host, org)"]
    B --> C["_resolve_token(host_info, org)"]
    C --> D["Step 1: ADO_APM_PAT / per-org PAT env var"]
    D -->|not found| E["Step 2: GITHUB_APM_PAT / GITHUB_TOKEN / GH_TOKEN"]
    E -->|not found| F["Step 3 NEW: resolve_credential_from_gh_cli(host)"]
    F --> G["subprocess: gh auth token --hostname host"]
    G -->|token valid| H["return gh_token, source=gh-auth-token"]
    G -->|fail/empty| I["Step 4: resolve_credential_from_git(host, port)"]
    I --> J["subprocess: git credential fill"]
    J -->|token found| K["return credential token"]
    J -->|not found| L["return None -> unauthenticated"]
    E -->|env token fails at remote| M["try_with_fallback detects failure"]
    M --> N{"host_info.kind in github/ghe_cloud/ghes?"}
    N -->|yes| O["resolve_credential_from_gh_cli(host)"]
    O -->|token| P["retry operation with gh token"]
    O -->|None| Q["resolve_credential_from_git fallback"]
    N -->|no/ado| Q
    Q -->|cred found| R["retry operation with git credential"]
    Q -->|not found| S["raise original exception"]
    style F fill:#fff3b0,stroke:#d47600
    style O fill:#fff3b0,stroke:#d47600

sequenceDiagram
    participant CLI as apm CLI
    participant AR as AuthResolver
    participant TM as GitHubTokenManager
    participant GH as gh CLI subprocess
    participant GIT as git credential subprocess

    CLI->>AR: resolve(host, org)
    AR->>AR: check PAT env vars (steps 1+2)
    AR->>TM: resolve_credential_from_gh_cli(host)
    TM->>GH: gh auth token --hostname host
    GH-->>TM: token or non-zero exit
    alt gh token valid
        TM-->>AR: token
        AR-->>CLI: AuthContext(source=gh-auth-token)
    else no gh token
        TM->>GIT: git credential fill
        GIT-->>TM: credential or empty
        TM-->>AR: credential or None
        AR-->>CLI: AuthContext or unauthenticated
    end

Recommendation

Merge after the doc-table break in apm-guide/.apm/skills/apm-usage/authentication.md is fixed (doc-writer blocking finding -- one-line move of the prose note after the complete table). The core auth behavior change is correct, the subprocess is structurally safe, and the zero-config story for gh CLI users is a net positive for APM adoption. File immediate follow-up issues for: (1) the three test-coverage gaps (autouse fixture disabling gh-CLI branch, missing _supports_gh_cli_host tests, missing source-string assertion), (2) the GHES guard unification and double-invocation fix in auth.py, and (3) the authentication.md table numbering fix and quickstart callout. The CHANGELOG entry should be framed as a user promise ('if you use gh, you are already set up') and a social beat should follow the merge.

_{This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.}

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by PR Review Panel for issue #630 · ● 3M · ◷

Acted on the panel review in commit 74d4e78 (pushed to fix/disambiguate-git-credential-fill):

Blocking (doc-writer)

• Fixed the orphaned table row + mid-table prose in packages/apm-guide/.apm/skills/apm-usage/authentication.md. Table is now contiguous (7 rows), prose moved below.

Code (auth-expert)

• Folded _supports_gh_cli_host into resolve_credential_from_gh_cli so both call sites (_resolve_token + _try_credential_fallback) share one eligibility path; closes the GHES-default-host divergence.
• Added 'gh-auth-token' to the _try_credential_fallback short-circuit set; no double-invoke when gh CLI was already the source.
• Hardened the gh subprocess: stdin=subprocess.DEVNULL, GH_NO_UPDATE_NOTIFIER=1, debug-log of stderr on non-zero exit.
• Per-step verbose logging ('trying gh auth token for X' then 'trying git credential fill for X') and updated docstring.

Tests (test-coverage-expert -- 3 gaps)

• TestSupportsGhCliHost: None / empty / ADO / github.com / *.ghe.com / GHES match / GHES mismatch / no GITHUB_HOST.
• test_gh_cli_source_label: asserts AuthContext.source == 'gh-auth-token' when gh CLI supplies the token.
• test_try_with_fallback_uses_gh_cli: exercises the gh-CLI branch through try_with_fallback, escaping the autouse disable fixture.
• Augmented existing TestResolveCredentialFromGhCli with assertions for stdin=DEVNULL + GH_NO_UPDATE_NOTIFIER=1, plus a guard test that ineligible hosts skip the subprocess entirely.

Docs (devx-ux + oss-growth-hacker)

• Reconciled prose-vs-table priority numbering in docs/.../authentication.md.
• Updated 'Package source behavior' table to mention gh auth token.
• Annotated mermaid F node 'GitHub-like hosts only'; added silent-skip note.
• Added gh CLI :::tip callout to quick-start.md.
• Reconciled stale '30s' timeout reference to '60s' to match DEFAULT_CREDENTIAL_TIMEOUT.

CHANGELOG

• Added entry under [Unreleased] -> Added framed as user promise, tagged (#630).

Verified

• uv run --extra dev ruff check src/ tests/ -- silent.
• uv run --extra dev ruff format --check src/ tests/ -- 709 files already formatted.
• uv run pytest tests/test_token_manager.py tests/unit/test_auth.py -q -- 140 passed.